fix(profiler): no such file /proc/{pid}/task #117
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for flashlight-docs canceled.
|
Almouro
force-pushed
the
fix/no-such-file
branch
from
e247f70
to
89fc8cf
Compare
Flashlight Automated Report 🔦 |
Almouro
force-pushed
the
fix/no-such-file
branch
2 times, most recently
from
4085dcd
to
dae63fa
Compare
Flashlight Automated Report 🔦 |
Flashlight Automated Report 🔦 |
Flashlight Automated Report 🔦 |
Almouro
changed the title
Flashlight Automated Report 🔦 |
Pid can change if app is restarted in the background
Flashlight Automated Report 🔦 |
niebo26
reviewed
Jul 11, 2023
| @@ -10,7 +12,7 @@ const mockSpawn = (): { stdout: EventEmitter; kill: () => void } => { | |||
| .spyOn(require("child_process"), "spawn") | |||
| .mockImplementationOnce((...args) => { | |||
| expect(args).toEqual(["adb", ["shell", "atrace", "-c", "view", "-t", "999"]]); | |||
| return mockProcess; | |||
| return mockProcess as ChildProcess; | |||
There was a problem hiding this comment.
Are we okay with these casts I suppose ?
legkikh
reviewed
Jul 11, 2023
| @@ -1,4 +1,6 @@ | |||
| /* eslint-disable @typescript-eslint/no-var-requires */ | |||
There was a problem hiding this comment.
yes (but at least it's in a test file)
niebo26
reviewed
Jul 11, 2023
| .spyOn(require("child_process"), "spawn") | ||
| .mockImplementationOnce((...args) => { | ||
| expect(args).toEqual(["adb", ["shell", "atrace", "-c", "view", "-t", "999"]]); | ||
| return aTraceMock as ChildProcess; | ||
| return aTraceMock; |
There was a problem hiding this comment.
Ah well i was wondering why the extra cast on the previous commit haha
niebo26
reviewed
Jul 11, 2023
niebo26
reviewed
Jul 11, 2023
| process.stderr?.on("data", (data) => { | ||
| const log = data.toString(); | ||
|
|
||
| // Ignore errors, it might be that the thread is dead and we can read stats anymore |
niebo26
approved these changes
Jul 11, 2023
Flashlight Automated Report 🔦 |
Almouro
added a commit
that referenced
this pull request
Jul 12, 2023
* fix(profiler): ensure failure if profiling script fails * docs(cpp): update docs to run c++ profiler * refactor(cpp): move logic to function * chore(cpp): throw custom error if directory not found * refactor: move some test utils to e2e folder * refactor(test): separate between atrace mock and profiler mock * refactor(test): fix some typings * refactor(profiler): add logStderr option * refactor(profiler): pass errors on stderr instead of stdout * fix(profiler): reset measures if pid changes Pid can change if app is restarted in the background * chore(cpp): recompile executables * fix(profiler): ensure graceful kill doesn't throw error * chore: fix comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested with APKPure apk and this command:
test.yamlisWith current
flashlight testcommand it used to show an error "no such file /proc/{pid}/task" in CLI and then actual measures of the app opened were lostWhat was happening was:
flashlight testmaestrorestarted the app with a new pidNow: